Modules with dagger 2 #8
Conversation
| // @JvmStatic | ||
| // @Provides | ||
| // @Singleton | ||
| // fun provideSharedPreferences(context: Context): SharedPreferences = |
There was a problem hiding this comment.
Закоменченный код лучше не добавлять в гит, это засоряет историю. Нужно либо выпилить сейчас, либо оставить провайдинг.
| // abstract fun getOnBoardingRepo(onBoardingRepoImpl: OnBoardingRepoImpl): OnBoardingRepo | ||
| // | ||
| // @IntoMap |
There was a problem hiding this comment.
То же самое: Закоменченный код лучше не добавлять в гит, это засоряет историю. Нужно либо выпилить сейчас, либо оставить провайдинг.
| # public *; | ||
| #} | ||
|
|
||
| # Uncomment this to preserve the line number information for |
There was a problem hiding this comment.
Эти файлы можно удалить, тут только закоменченный код, в текущем варианте он бесполезен, только засоряет историю.
| @@ -48,12 +45,19 @@ abstract class BaseActivity: AppCompatActivity(), SubscriptionsHolder { | |||
| } | |||
| } | |||
|
|
|||
| /** Для использования в feature модулях */ | |||
| private fun initLayoutFromConstructor() { | |||
There was a problem hiding this comment.
Плохо, когда у тебя есть несколько вариантов провайдинга чего-то в одном базовом классе. Класс становится больше и потом разобраться сложнее намного что происходит. В данном случае лучше всего оставить один вариант указания макета, второй убрать, либо делать на несколько базовых классов.
| @@ -29,7 +27,7 @@ abstract class BaseFragment: Fragment() { | |||
|
|
|||
| protected abstract fun initUx() | |||
| protected abstract fun initUi() | |||
| open fun showProgress(show: Boolean) = progress?.visible(show) | |||
| // open fun showProgress(show: Boolean) = progress?.visible(show) | |||
| class OnBoardingViewModel @Inject constructor( | ||
| application: Application, | ||
| private val getOnboardingVisibilityInteractor: GetOnboardingVisibilityInteractor, | ||
| private val onBoardingRepo: OnBoardingRepo |
There was a problem hiding this comment.
Тот же самый коммент про сокращение названий - лучше так не делать
| import javax.inject.Inject | ||
|
|
||
| class OnBoardingViewModel @Inject constructor( | ||
| application: Application, |
There was a problem hiding this comment.
Лучше не использовать контекст во вьюмоделях, потому что тестировать будет потом это проблематично.
Показывать сообщения лучше либо через какой-то хелпер класс, либо напрямую через View. Тот же снекбар показать через ApplicationContext невозможно будет
| @@ -8,7 +8,7 @@ | |||
| android:layout_width="match_parent" | |||
| android:layout_height="match_parent" | |||
| android:layout_marginTop="@dimen/small_margin" | |||
| android:background="@color/white" /> | |||
| android:background="@android:color/white" /> | |||
There was a problem hiding this comment.
лучше использовать свои собственные цвета, потому что некоторые устройства меняют дефолтные цвета, и на каком-нибудь Самсунге цвет будет не тот, который задуман дизайнером
| api fragmentKtx | ||
|
|
||
| // Dagger 2 | ||
| api dagger2.core |
There was a problem hiding this comment.
Не очень понятно зачем тут все делать через api. Надо бы пройтись по ним и поправить. https://medium.com/mindorks/implementation-vs-api-in-gradle-3-0-494c817a6fa
| @@ -1 +1 @@ | |||
| include ':app', ':domain' | |||
| include ':app', ':domain', ':features:feature_onboarding', ':base:core' | |||
There was a problem hiding this comment.
Опасно делать такой core модуль, потому что туда можно будет запихать всё что угодно. Лучше поделить его на несколько модулей поменьше. Вроде uicore, domaincore, networkcore и т.д. Либо просто положить эти утилиты в network, domain, ui если не планируется разбиения на более мелкие модули
No description provided.